Hey @sponce can you have a look? I think the render size check in MakePictureComponent would still stop us clicking the button? But if I comment this out, I seem to get a black screen (Windows 10, firefox)

Hi @EdwardMoyse and @sponce ,

I’ve updated the PR:
Removed the old checkScreenShotCanvasSize logic from MakePictureComponent
Removed the code that was disabling the “Create Picture” button
Since the tiled screenshot implementation now supports arbitrarily large images, the size-check is no longer needed
The button remains enabled and the tiling system handles large screenshots correctly.

Please let me know if you want any more adjustments.
Thanks!

Thanks for the cleanup of checksize and disabled. Now there was more in the "change of logic" I was mentioning. This is about these lines for example :

const scaledSize = this.croppedSize(
      width,
      height,
      originalSize.width,
      originalSize.height,
    );
    const heightShift = (scaledSize.height - height) / 2;
    const widthShift = (scaledSize.width - width) / 2;

What is was doing is to deal with the scaling factor between the screen (say 1920x1440 as an example) and the final picture (let's suppose 6000*4000). The ratio is not the same (1.33 vs 1.5) so in case the user chose the crop option, we'll have to add empty bards on the left and right. This is handled via the Shift variables.
In case the user chose the Stretch option, I was expecting to have a scaleWidth and scaleHeight independent, but I see it's not there. Did we break that option ?

I think all that is gone now and instead of only showing what is on the screen with potential empty bars, you showing more than the screen. Am I correct ?

Thanks for the detailed explanation! @sponce Yes — after reviewing my changes and your original implementation, I now understand the issue.

You are correct: by removing the scaling/shift logic and only using tiling + setViewOffset, the current version is effectively rendering more than the visible screen whenever the requested aspect ratio differs from the viewport ratio.
So yes — the Crop/Stretch behavior is currently broken:
• Crop: previously added bars using widthShift / heightShift
• Stretch: previously applied independent X/Y scaling
• Now both behave like “extend view beyond screen”, which is not intended.

I will restore the original ratio-handling logic:
Re-implement croppedSize() usage
Re-add widthShift & heightShift
Re-apply correct scale logic for Stretch mode

I’ll push an updated commit shortly.

Thanks a lot for that. Looks all good from a quick code inspection.
Now when trying it, the crop is perfect but the strech is somehow not working. See the 3 files attached :

• number 6 gives an idea of what I have on the screen (it's also cropped, but not too much)
• number 4 is the cropped version to a "flat" image, and worked nicely
• number 5 is the streched version to the same size and is not what is expected.

Thanks for the detailed visual example @sponce — it helped identify the issue.
Stretch mode was failing because the camera aspect ratio was never updated. I added logic to temporarily set camera.aspect = width / height during the screenshot and restore it afterward. Now Stretch produces the correct full-frame output.

I wanted to try it, but it does not compile for me :

- Generating browser application bundles (phase: setup)...
src/managers/three-manager/index.ts(1448,7): error TS2322: Type 'unknown' is not assignable to type 'number'.
src/managers/three-manager/index.ts(1450,14): error TS2551: Property 'updateProjectionMatrix' does not exist on type 'Camera & Record<"aspect", unknown>'. Did you mean 'projectionMatrix'?
src/managers/three-manager/index.ts(1529,14): error TS2339: Property 'aspect' does not exist on type 'Camera'.
src/managers/three-manager/index.ts(1530,14): error TS2551: Property 'updateProjectionMatrix' does not exist on type 'Camera'. Did you mean 'projectionMatrix'?
5:41:43 PM - Found 4 errors. Watching for file changes.

I fetched the latest version of your branch. Did you not push the right thing ?

Thanks for the feedback! @sponce
I’ve updated the implementation to fully restore the original crop/stretch logic, added safe PerspectiveCamera checks, fixed the projection matrix updates, and resolved all TypeScript errors..
Please try it again and let me know if any further adjustments are needed.

I still have the stretched version not looking as expected :

Hi @sponce ,
I’ve now fully separated Crop vs Stretch paths, removed all shift logic for Stretch, ensured correct viewOffset bounds, and handled PerspectiveCamera aspect updates safely.
The updated implementation should now produce the exact expected stretched output.
Please try again and let me know if any further adjustments are needed.

Here is what I get now for Stretch :

I'm not sure whether I'm happy or not. To be honest I was expecting something like this :

This was the previous implementation, but on the other hand I do not really see the interest of it, while your version definitely makes sense. So I would say we leave it like this

Thanks again for the good work !

Thanks a lot for reviewing @sponce
I’m glad the current implementation makes sense.
If any further adjustments are needed later, I’ll be happy to look into them.
Thanks again for your guidance throughout the PR!

@EdwardMoyse the ci is failing, but it looks like it's the machinery itself, not the PR. Am I correct ?

@EdwardMoyse the ci is failing, but it looks like it's the machinery itself, not the PR. Am I correct ?

Not AFAIK. Other MRs are passing CI ... somehow it is related to this one.

Hi @deveshbervar - does yarn test:ci work for you locally? I'll try to look into this later, but I'm not sure why you need to update the dockerfile for this MR, when it's running fine in all the other CIs (though I have to admit I've not had time to look properly yet so perhaps I'm missing something obvious?)

Hi @EdwardMoyse, thanks for the review!
I reverted the callback typings back to MouseEvent and kept them initialized with no-op functions to avoid the TS “callable {}” error.
I also updated the Dockerfile by replacing python with python3, which fixes the CI package installation issue on Ubuntu 22.04.
Let me know if anything else needs adjustment!